Patchwork [RFC] WORDS_BIG_ENDIAN != BYTES_BIG_ENDIAN messes up subreg

login
register
mail settings
Submitter Paul Koning
Date Oct. 12, 2010, 9:31 p.m.
Message ID <8CD69747-312C-4580-B85B-5252CAFCB29E@dell.com>
Download mbox | patch
Permalink /patch/67619/
State New
Headers show

Comments

Paul Koning - Oct. 12, 2010, 9:31 p.m.
I just ran into an issue when BYTES_BIG_ENDIAN differs from WORDS_BIG_ENDIAN, which apparently at this point applies only to pdp11.

The problem is that I end up with HI mode subreg references to odd bytes, which cause a gcc_assert.  It appears to me that this patch is needed.

I'm not all that confident about my analysis, but the change does cure the assert and as far as I can tell it won't affect other platforms.  Also, it seems to make sense given the definition of what those macros do -- assuming I understood them correctly.

Comments appreciated.

	paul
Richard Henderson - Oct. 12, 2010, 9:41 p.m.
On 10/12/2010 02:31 PM, Paul Koning wrote:
> I just ran into an issue when BYTES_BIG_ENDIAN differs from
> WORDS_BIG_ENDIAN, which apparently at this point applies only to
> pdp11.

Yep, it would do.  Can you point to an exemplar -- preferably with
a bug report for tracking -- that hits this?

I have a feeling that while your patch works for HImode, it'll 
fail for a QImode subreg of an SImode value.  Which means that
we'll need some other adjustment in there instead.


r~
Paul Koning - Oct. 13, 2010, 12:56 a.m.
On Oct 12, 2010, at 5:41 PM, Richard Henderson wrote:

> On 10/12/2010 02:31 PM, Paul Koning wrote:
>> I just ran into an issue when BYTES_BIG_ENDIAN differs from
>> WORDS_BIG_ENDIAN, which apparently at this point applies only to
>> pdp11.
> 
> Yep, it would do.  Can you point to an exemplar -- preferably with
> a bug report for tracking -- that hits this?

I hit it in compiling one of the pieces of libgcc2.c.  Will see about reducing it to a small example.

> 
> I have a feeling that while your patch works for HImode, it'll 
> fail for a QImode subreg of an SImode value.  Which means that
> we'll need some other adjustment in there instead.

The code in question deals with shifts of multiword quantities, for example a shift of a DImode value where the registers are smaller than that (HImode in the pdp11).  I don't think the case you mentioned would occur, because the subreg mode is HImode since the registers are 16 bits.

	paul
Richard Henderson - Oct. 13, 2010, 5:06 p.m.
On 10/12/2010 05:56 PM, Paul Koning wrote:
> The code in question deals with shifts of multiword quantities, for
> example a shift of a DImode value where the registers are smaller
> than that (HImode in the pdp11). I don't think the case you mentioned
> would occur, because the subreg mode is HImode since the registers
> are 16 bits.

Err, quite right.  I should have looked closer.

The patch is ok.  Referencing BYTES_BIG_ENDIAN here is
clearly irrelevant to manipulating words.


r~

Patch

Index: lower-subreg.c
===================================================================
--- lower-subreg.c	(revision 165191)
+++ lower-subreg.c	(working copy)
@@ -1008,13 +1008,6 @@ 
   offset2 = UNITS_PER_WORD * (1 - dest_reg_num);
   src_offset = UNITS_PER_WORD * src_reg_num;
 
-  if (WORDS_BIG_ENDIAN != BYTES_BIG_ENDIAN)
-    {
-      offset1 += UNITS_PER_WORD - 1;
-      offset2 += UNITS_PER_WORD - 1;
-      src_offset += UNITS_PER_WORD - 1;
-    }
-
   start_sequence ();
 
   dest_reg = simplify_gen_subreg_concatn (word_mode, SET_DEST (set),