[PATCH, RFC] PR 55403 + 55391

[Richard Sandiford]

Sorry, I realised when posting the expmed patch earlier today that I hadn't
finished this off.

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Richard Henderson <rth@redhat.com> writes:
>> On 11/20/2012 11:24 AM, Richard Henderson wrote:
>>> On 11/20/2012 09:29 AM, Richard Sandiford wrote:
>>>> Gah.  How about this patch, currently bootstrapping on x86_64-linux-gnu
>>>> as a sanity check?  The last instance seems glaringly obvious in
>>>> hindsight :-(
>>>>
>>>> Richard
>>>>
>>>>
>>>> gcc/
>>>> 	* expmed.c (store_bit_field_1): Use adjust_bitfield_address_size
>>>> 	rather than adjust_bitfield_address to change the mode of a reference.
>>>> 	(extract_bit_field_1): Likewise.
>>> 
>>> That patch does fix my ICE.
>>> 
>>> It looks all good, too.  As you say, glaringly obvious even.  ;-)
>>
>> One further point -- get_best_mem_extraction_insn does not work for
>> the traditional 'extv' patterns that only accept memories.  In
>> particular, the QImode memory in the extv pattern never matches up
>> at the beginning of get_traditional_extraction_insn, so that first
>> "if (mode != struct_mode) return false;" always triggers.
>
> Sorry for all the mishaps.
>
>> I audited the existing extv patterns and the affected targets are
>> alpha, sh, and vax.  All of the others implement extv on registers,
>> which appears to work.  Test case:
>>
>> struct S { long y __attribute__((packed)); };
>> long g(struct S *s) { return s->y; }
>>
>> Before:
>>
>> 	ldq_u $0,0($16)
>> 	ldq_u $1,7($16)
>> 	extql $0,$16,$0
>> 	extqh $1,$16,$16
>> 	bis $0,$16,$0
>>
>> After:
>>
>> 	ldbu $5,1($16)
>> 	ldbu $8,0($16)
>> 	ldbu $7,2($16)
>> 	ldbu $6,3($16)
>> 	ldbu $4,4($16)
>> 	ldbu $3,5($16)
>> 	sll $5,8,$5
>> 	ldbu $2,6($16)
>> 	ldbu $1,7($16)
>> 	sll $7,16,$7
>> 	sll $6,24,$6
>> 	bis $5,$8,$5
>> 	sll $4,32,$4
>> 	sll $3,40,$3
>> 	bis $7,$5,$5
>> 	sll $2,48,$2
>> 	sll $1,56,$1
>> 	bis $6,$5,$0
>> 	bis $4,$0,$0
>> 	bis $3,$0,$0
>> 	bis $2,$0,$0
>> 	bis $1,$0,$0
>>
>> I suppose the question is: with only 3 affected targets, is it more
>> trouble fiddling the somewhat confused "traditional" path, or to
>> just go ahead and update the backends?
>
> I suppose updating them would be the ideal eventually, but I'd still
> like to fix the bug.
>
> I belatedly did a similar audit and it looks there are no patterns that
> use a mixture of field and structure modes for register operations.
> They're either equal or not present.

I was wrong, i386.md has an extzv that allows SImode extractions of
DImode registers.  I tested the patch below at the time of the original
thread but for some reason didn't get around to posting it.

I've just rechecked that it doesn't change the output of gcc .ii
files for x86_64-linux-gnu and that it fixes your testcase for SH4A.
Retested on x86_64-linux-gnu and mips-sde-elf.  OK to install?

Richard


gcc/
	* optabs.c (get_traditional_extraction_insn): Check the field mode
	rather than the structure mode for memory operations.

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	2013-01-31 09:17:21.551473246 +0000
+++ gcc/optabs.c	2013-01-31 09:27:35.535226561 +0000
@@ -8261,23 +8261,35 @@ get_traditional_extraction_insn (extract
 {
   const struct insn_data_d *data = &insn_data[icode];
 
-  enum machine_mode struct_mode = data->operand[struct_op].mode;
-  if (struct_mode == VOIDmode)
-    struct_mode = word_mode;
-  if (mode != struct_mode)
-    return false;
-
   enum machine_mode field_mode = data->operand[field_op].mode;
   if (field_mode == VOIDmode)
     field_mode = word_mode;
 
+  enum machine_mode struct_mode;
+  if (type == ET_unaligned_mem)
+    {
+      /* Memory structure operands refer to the first byte of the
+	 bitfield and the true mode is taken from the field operand.  */
+      struct_mode = byte_mode;
+      if (mode != field_mode)
+	return false;
+    }
+  else
+    {
+      struct_mode = data->operand[struct_op].mode;
+      if (struct_mode == VOIDmode)
+	struct_mode = word_mode;
+      if (mode != struct_mode)
+	return false;
+    }
+
   enum machine_mode pos_mode = data->operand[struct_op + 2].mode;
   if (pos_mode == VOIDmode)
     pos_mode = word_mode;
 
   insn->icode = icode;
   insn->field_mode = field_mode;
-  insn->struct_mode = (type == ET_unaligned_mem ? byte_mode : struct_mode);
+  insn->struct_mode = struct_mode;
   insn->pos_mode = pos_mode;
   return true;
 }