This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix for "FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal compiler error)"


On 09/05/2014 10:51 AM, David Sherwood wrote:
> Hi,
>
> I have a potential fix for a gcc testsuite failure for aarch64 in big
> endian, i.e.
>
> FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal
> compiler error)
> FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_y_tst.o compile, (internal
> compiler error)
>
> It is caused by the inappropriate choice of hard registers for paradoxical
> sub registers in
> big endian mode, for example if register 0 is chosen for a paradoxical TI
> subreg on big
> endian then we may end up attempting to reference register -1. Similarly, on
> little endian
> we could end up going beyond the upper bounds of the register file too.
>
> My fix involves adding particular constraints in IRA on the choice of
> register once paradoxical 
> sub registers are encountered. However, Richard Sandiford also proposed an
> alternative
> solution that involves not constraining registers in IRA, but rather making
> use of cost analysis
> instead and letting LRA do the work. Not sure what your preference is ....
Richard's proposal could work too but I prefer your solution because it
directly prevents occurring such situations and the most important sets
up real conflicting hard regs which results in more accurate choice of
dynamic allocno class in IRA and as a result improves coloring in IRA.

The patch is ok (we can not use reg_max_ref_width as it is defined only
in reload).

Could you just change the following line

              if (NULL != parent && GET_CODE (parent) == SUBREG)

 to

              if (parent != NULL && GET_CODE (parent) == SUBREG)

It is very unusual to see such code in GCC.

I see a missed blank too after the comma.  As I remeber we need a blank
in such case according to GNU coding standard.

outer_mode,outer_regno


I'd also use name 'outer' instead of 'parent' as it usual naming for
this.  But it is just a matter of taste.
 
Also as it is your first patch, please test it thoroughly (bootstrap and
check-gcc with comparison of results before and after the patch) on
major platforms available to you (at least x86/x86-64).

If it is ok, you can commit the patch.

Thanks for working on the failure.

> Fix was tested on aarch64 on little and big endian with no regressions.
>
> Regards,
> David Sherwood.
>
> 2014-08-26  David Sherwood  <david.sherwood@arm.com>
>
>         * ira-int.h (ira_allocno): Add "wmode" field.
>         * ira-build.c (create_insn_allocnos): Add new "parent" function
>         parameter.
>         * ira-conflicts.c (ira_build_conflicts): Add conflicts for registers
>         that cannot be accessed in wmode.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]