diff mbox

[PR,64192] Add forgotten conversion from bits to bytes

Message ID 20141205161332.GX8214@virgil.suse
State New
Headers show

Commit Message

Martin Jambor Dec. 5, 2014, 4:13 p.m. UTC
Hi,

at some point I lost an important division of bit offset by
BITS_PER_UNIT in my alignment IPA-CP propagation patch. That lead to a
few failures on i686 reported as PR 64192.

This patch adds it together with a slight improvement of the guarding
check which I suppose will never trigger but it does ensure the
division will never loose information.

I consider this change obvious and would really like to commit it
before I leave for the weekend, so I will do so after it finishes
bootstrapping and testing on i686.  It has already passed bootstrap
and testing on x86_64-linux.

Thanks,

Martin


2014-12-05  Martin Jambor  <mjambor@suse.cz>

	PR ipa/64192
	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Convert alignment
	from bits to bytes after checking they are byte-aligned.

Comments

H.J. Lu Dec. 5, 2014, 5:51 p.m. UTC | #1
On Fri, Dec 5, 2014 at 8:13 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> at some point I lost an important division of bit offset by
> BITS_PER_UNIT in my alignment IPA-CP propagation patch. That lead to a
> few failures on i686 reported as PR 64192.
>
> This patch adds it together with a slight improvement of the guarding
> check which I suppose will never trigger but it does ensure the
> division will never loose information.
>
> I consider this change obvious and would really like to commit it
> before I leave for the weekend, so I will do so after it finishes
> bootstrapping and testing on i686.  It has already passed bootstrap
> and testing on x86_64-linux.
>
> Thanks,
>
> Martin
>
>
> 2014-12-05  Martin Jambor  <mjambor@suse.cz>
>
>         PR ipa/64192
>         * ipa-prop.c (ipa_compute_jump_functions_for_edge): Convert alignment
>         from bits to bytes after checking they are byte-aligned.
>
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -1739,10 +1739,11 @@ ipa_compute_jump_functions_for_edge (str
>           unsigned align;
>
>           if (get_pointer_alignment_1 (arg, &align, &hwi_bitpos)
> -             && align > BITS_PER_UNIT)
> +             && align % BITS_PER_UNIT == 0
> +             && hwi_bitpos % BITS_PER_UNIT == 0)
>             {
>               jfunc->alignment.known = true;
> -             jfunc->alignment.align = align;
> +             jfunc->alignment.align = align / BITS_PER_UNIT;
>               jfunc->alignment.misalign = hwi_bitpos / BITS_PER_UNIT;
>             }
>           else

It also fixed SPEC CPU 2000 regression on Linux/i686.

Thanks.
Jeff Law Dec. 5, 2014, 8:46 p.m. UTC | #2
On 12/05/14 09:13, Martin Jambor wrote:
> Hi,
>
> at some point I lost an important division of bit offset by
> BITS_PER_UNIT in my alignment IPA-CP propagation patch. That lead to a
> few failures on i686 reported as PR 64192.
>
> This patch adds it together with a slight improvement of the guarding
> check which I suppose will never trigger but it does ensure the
> division will never loose information.
>
> I consider this change obvious and would really like to commit it
> before I leave for the weekend, so I will do so after it finishes
> bootstrapping and testing on i686.  It has already passed bootstrap
> and testing on x86_64-linux.
>
> Thanks,
>
> Martin
>
>
> 2014-12-05  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/64192
> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Convert alignment
> 	from bits to bytes after checking they are byte-aligned.
OK.

If you could add a testcase when you get back that'd be appreciated.

jeff
H.J. Lu Dec. 5, 2014, 8:58 p.m. UTC | #3
On Fri, Dec 5, 2014 at 12:46 PM, Jeff Law <law@redhat.com> wrote:
> On 12/05/14 09:13, Martin Jambor wrote:
>>
>> Hi,
>>
>> at some point I lost an important division of bit offset by
>> BITS_PER_UNIT in my alignment IPA-CP propagation patch. That lead to a
>> few failures on i686 reported as PR 64192.
>>
>> This patch adds it together with a slight improvement of the guarding
>> check which I suppose will never trigger but it does ensure the
>> division will never loose information.
>>
>> I consider this change obvious and would really like to commit it
>> before I leave for the weekend, so I will do so after it finishes
>> bootstrapping and testing on i686.  It has already passed bootstrap
>> and testing on x86_64-linux.
>>
>> Thanks,
>>
>> Martin
>>
>>
>> 2014-12-05  Martin Jambor  <mjambor@suse.cz>
>>
>>         PR ipa/64192
>>         * ipa-prop.c (ipa_compute_jump_functions_for_edge): Convert
>> alignment
>>         from bits to bytes after checking they are byte-aligned.
>
> OK.
>
> If you could add a testcase when you get back that'd be appreciated.
>

It is covered by existing testcases.  Those failed on Linux/i686:

FAIL: gcc.c-torture/execute/pr37573.c   -O3 -fomit-frame-pointer  execution test
FAIL: gcc.c-torture/execute/pr37573.c   -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr37573.c   -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gcc.c-torture/execute/pr37573.c   -O3 -g  execution test
FAIL: gcc.dg/vect/pr60196-1.c execution test
FAIL: gcc.dg/vect/pr60196-1.c -flto -ffat-lto-objects execution test
FAIL: gcc.dg/vect/vect-multitypes-11.c -flto -ffat-lto-objects execution test
FAIL: gcc.dg/vect/vect-multitypes-12.c -flto -ffat-lto-objects execution test
diff mbox

Patch

Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -1739,10 +1739,11 @@  ipa_compute_jump_functions_for_edge (str
 	  unsigned align;
 
 	  if (get_pointer_alignment_1 (arg, &align, &hwi_bitpos)
-	      && align > BITS_PER_UNIT)
+	      && align % BITS_PER_UNIT == 0
+	      && hwi_bitpos % BITS_PER_UNIT == 0)
 	    {
 	      jfunc->alignment.known = true;
-	      jfunc->alignment.align = align;
+	      jfunc->alignment.align = align / BITS_PER_UNIT;
 	      jfunc->alignment.misalign = hwi_bitpos / BITS_PER_UNIT;
 	    }
 	  else