Bug 51893 - Wrong subword index computation in store_bit_field_1 on BIG_ENDIAN targets
Summary: Wrong subword index computation in store_bit_field_1 on BIG_ENDIAN targets
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.6.1
: P3 normal
Target Milestone: 4.7.1
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-01-18 17:13 UTC by Aurelien Buhrig
Modified: 2012-03-28 09:10 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-01-19 00:00:00


Attachments
Patch (318 bytes, patch)
2012-01-18 17:13 UTC, Aurelien Buhrig
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aurelien Buhrig 2012-01-18 17:13:09 UTC
Created attachment 26369 [details]
Patch

The subword index (wordnum) passed to operand_subword_force in store_bit_field_1 is incorrect for BIG_ENDIAN targets when bitsize > BITS_PER_WORD.

wordnum, which is used to get the subword of value, is
computed wrt the number of words needed by bitsize instead of the number of words needed by the integer mode of the field, which is the mode used to compute subwords.

For instance, for a 40bit field to be set by a DI reg (with HI words),
the offset of the LSWord of the DI reg should be 3, not 2 as currently
computed.

The attached patch seems to solve the issue. Tested on the C testsuite
without any regression (for my target only).

Note that this problem occurs on a "private" target (hardly reproducible).
Comment 1 Andrew Pinski 2012-01-18 23:06:20 UTC
I see the issue, one place does it right and the other does not.
Comment 2 Richard Biener 2012-01-19 10:11:49 UTC
So, confirmed.
Comment 3 Eric Botcazou 2012-01-19 10:47:45 UTC
Please try the fix for PR middle-end/50325.
Comment 4 Aurelien Buhrig 2012-01-20 09:00:38 UTC
After modifying this patch for 4.6.1 this patch doesn't work (bitfld-3.c testcase).

It doesn't affect the value subword offset computation (wordnum) when calling operand_subword_force.
Comment 5 Eric Botcazou 2012-01-20 10:04:50 UTC
> After modifying this patch for 4.6.1 this patch doesn't work (bitfld-3.c
> testcase).

What do you mean exactly?  That gnat.dg/bitfld-3.c fails with the patch?
Comment 6 Aurelien Buhrig 2012-01-20 10:32:22 UTC
(In reply to comment #5)
> > After modifying this patch for 4.6.1 this patch doesn't work (bitfld-3.c
> > testcase).
> 
> What do you mean exactly?  That gnat.dg/bitfld-3.c fails with the patch?

I mean the patch was likely done for 4.7, and I made very small adjustments to enable patching on 4.6.1.

The testcase that fails is gcc.c-torture/execute/bitfld-3.c. Both with and without (4.6.1 release) the patch. 
The patch I posted fixes the problem, but I don't know if it is general enough.
Comment 7 Eric Botcazou 2012-01-20 11:28:26 UTC
> The testcase that fails is gcc.c-torture/execute/bitfld-3.c. Both with and
> without (4.6.1 release) the patch. 
> The patch I posted fixes the problem, but I don't know if it is general enough.

OK, what are the values of the various parameters you have upon entering the problematic block of code in store_bit_field_1?  Note that this code has been working fine for years on 32-bit big-endian targets so this is unexpected.
Comment 8 Aurelien Buhrig 2012-01-23 08:27:22 UTC
It seems the problem occurs with big endian targets when value is at least 4 times bigger than a word.
Example:
bitsize=40, value = reg:DI sub words-->HI. So wordnum = 3.

The for loop should start from LSWord to bitsize MSWord (wordnum).
So:
subword 3 from value, and calls  store_bit_fields_1 with bitsize <= BITS_PER_WORD
then subword 2,
then subword 1
And subword 0 from value should be ignored.

Currently, the loop does not begins with subword 3, but with subword "wordnum-1" which is 2. It is not the LSword of value, and the value stored in the bitfield is (value << BITS_PER_WORD).
Comment 9 Aurelien Buhrig 2012-03-20 16:22:52 UTC
Do you need additional information about this bug?
Any comment about the provided patch?
Comment 10 Eric Botcazou 2012-03-20 22:03:27 UTC
> Do you need additional information about this bug?
> Any comment about the provided patch?

No, I think the patch is correct.  Please post it on gcc-patches@.
Comment 11 Eric Botcazou 2012-03-27 20:50:21 UTC
Author: ebotcazou
Date: Tue Mar 27 20:50:16 2012
New Revision: 185897

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185897
Log:
	PR middle-end/51893
	* expmed.c (store_bit_field_1): Fix wordnum value for big-endian
	targets.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expmed.c
Comment 12 Eric Botcazou 2012-03-28 09:06:30 UTC
Author: ebotcazou
Date: Wed Mar 28 09:06:03 2012
New Revision: 185909

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185909
Log:
	PR middle-end/51893
	* expmed.c (store_bit_field_1): Fix wordnum value for big-endian
	targets.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/expmed.c
Comment 13 Eric Botcazou 2012-03-28 09:10:35 UTC
Fixed in the 4.7.x series.  Thanks for the bug report and the patch.