| Message ID | 87pq38uqr3.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com |
|---|---|
| State | New |
| Headers | show |
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. ;-) r~
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. 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? r~
Index: gcc/expmed.c =================================================================== --- gcc/expmed.c 2012-11-20 09:55:26.000000000 +0000 +++ gcc/expmed.c 2012-11-20 17:24:09.722871322 +0000 @@ -645,7 +645,7 @@ store_bit_field_1 (rtx str_rtx, unsigned if (imode != GET_MODE (op0)) { if (MEM_P (op0)) - op0 = adjust_bitfield_address (op0, imode, 0); + op0 = adjust_bitfield_address_size (op0, imode, 0, MEM_SIZE (op0)); else { gcc_assert (imode != BLKmode); @@ -1380,7 +1380,7 @@ extract_bit_field_1 (rtx str_rtx, unsign if (imode != GET_MODE (op0)) { if (MEM_P (op0)) - op0 = adjust_bitfield_address (op0, imode, 0); + op0 = adjust_bitfield_address_size (op0, imode, 0, MEM_SIZE (op0)); else if (imode != BLKmode) { op0 = gen_lowpart (imode, op0); @@ -1403,10 +1403,10 @@ extract_bit_field_1 (rtx str_rtx, unsign } else { - rtx mem = assign_stack_temp (GET_MODE (op0), - GET_MODE_SIZE (GET_MODE (op0))); + HOST_WIDE_INT size = GET_MODE_SIZE (GET_MODE (op0)); + rtx mem = assign_stack_temp (GET_MODE (op0), size); emit_move_insn (mem, op0); - op0 = adjust_bitfield_address (mem, BLKmode, 0); + op0 = adjust_bitfield_address_size (mem, BLKmode, 0, size); } } }