On 1/8/19 8:21 AM, Andre Vieira (lists) wrote: > > > On 07/01/2019 22:50, Jeff Law wrote: >> On 1/7/19 7:42 AM, Andre Vieira (lists) wrote: >>> Hi, >>> >>> This patch fixes the way 'uses_hard_regs_p' handles paradoxical subregs. >>> The function is supposed to detect whether a register access of 'x' >>> overlaps with 'set'. For SUBREGs it should check whether any of the >>> full multi-register overlaps with 'set'. The former behavior used to >>> grab the widest mode of the inner/outer registers of a SUBREG and the >>> inner register, and check all registers from the inner-register onwards >>> for the given width. For normal SUBREGS this gives you the full >>> register, for paradoxical SUBREGS however it may give you the wrong set >>> of registers if the index is not the first of the multi-register set. >>> >>> The original error reported in PR target/86487 can no longer be >>> reproduced with the given test, this was due to an unrelated code-gen >>> change, regardless I believe this should still be fixed as it is simply >>> wrong behavior by uses_hard_regs_p which may be triggered by a different >>> test-case or by future changes to the compiler. Also it is useful to >>> point out that this isn't actually a 'target' issue as this code could >>> potentially hit any other target using paradoxical SUBREGS. Should I >>> change the Bugzilla ticket to reflect this is actually a target agnostic >>> issue in RTL? >>> >>> There is a gotcha here, I don't know what would happen if you hit the >>> cases of get_hard_regno where it would return -1, quoting the comment >>> above that function "If X is not a register or a subreg of a register, >>> return -1." though I think if we are hitting this then things must have >>> gone wrong before? >>> >>> Bootstrapped on aarch64, arm and x86, no regressions. >>> >>> Is this OK for trunk? >>> >>> >>> gcc/ChangeLog: >>> 2019-01-07 Andre Vieira <andre.simoesdiasvie...@arm.com> >>> >>> >>> PR target/86487 >>> * lra-constraints.c(uses_hard_regs_p): Fix handling of >>> paradoxical SUBREGS. >> But doesn't wider_subreg_mode give us the wider of the two modes here >> and we use that wider mode when we call overlaps_hard_reg_set_p which >> should ultimately check all the registers in the paradoxical. >> >> I must be missing something here?!? >> >> jeff >> > > Hi Jeff, > > It does give us the wider of the two modes, but we also then grab the > "subreg" of the paradoxical subreg. If you look at the first example > case of the bugzilla ticket, for an older gcc (say gcc-8) and the > options provided (using big-endian), it will generate the following subreg: > (subreg:DI (reg:SI 122) 0) > > This paradoxical subreg represents a register pair r0-r1, where because > of big-endian and subgreg index 0, r1 is the value we care about and r0 > the one we say "it can be whatever" by using this paradoxical subreg. > > When 'uses_hard_regs_p' sees this as a subreg, it sets 'mode' to the > wider, i.e. DImode, but it also sets 'x' to the subreg i.e. 'reg:SI > 122', for which get_hard_regno correctly returns 'r1'. But if you now > pass 'overlaps_hard_reg_set_p' DImode and 'r1', it will check whether > 'set' contains either 'r1-r2', and not 'r1'r0'. > > To reproduce this again I now applied this patch to GCC 8 and found an > issue with it. 'REG_P (x)' returns false if x is a 'SUBREG'. So I will > need to change the later check to also include 'SUBREG_P (x)', I guess I > was testing with a too new version of gcc that didn't lead to the bogus > register allocation... > > Which really encourages me to add some sort of testcase, but I'd very > much like to come up with a less flaky one, we basically need to force > the generation of a paradoxical subreg 'x', where 'get_hard_regno > (SUBREG_REG (x)) != get_hard_regno (x)'. This will cause > 'uses_hard_regs_p' to give you a wrong answer. BTW, you might look at 87305 which is another problem with big endian pseudos and paradoxical subregs that Vlad just fixed.
jeff
