On 18 Jul 2011, at 07:15, "Chung-Lin Tang" <clt...@codesourcery.com> wrote:
> On 2011/6/15 09:12 PM, Richard Earnshaw wrote: >> On 22/04/11 16:21, Chung-Lin Tang wrote: >>> Hi Richard, this part's for you. >>> >>> The ARM backend changes needed are very little after the prior patches, >>> basically just a case in arm_canonicalize_comparison() to detect >>> (zero_extend:SI (subreg:QI (reg:SI ...) 0)), and swap it into (and:SI >>> (reg:SI) #255). >>> >>> Had we not tried the combine modifications, this testcase probably could >>> have also be solved by implementing another version of the corresponding >>> *andsi3_compare0/_scratch patterns, with ZERO_EXTEND in the body, and >>> "ands" in the output assembly. Maybe that's an acceptable solution too... >>> >>> About the (ab)use of CANONICALIZE_COMPARISON, if it really should be >>> another macro/hook, then this ARM patch will need updating, but the code >>> should be similar. >>> >>> Thanks, >>> Chung-Lin >>> >>> >>> 3-arm-parts.diff >>> >>> >>> Index: config/arm/arm.c >>> =================================================================== >>> --- config/arm/arm.c (revision 172860) >>> +++ config/arm/arm.c (working copy) >>> @@ -3276,6 +3276,19 @@ >>> return code; >>> } >>> >>> + /* If *op0 is (zero_extend:SI (subreg:QI (reg:SI) 0)) and comparing >>> + with const0_rtx, change it to (and:SI (reg:SI) (const_int 255)), >>> + to facilitate possible combining with a cmp into 'ands'. */ >>> + if (mode == SImode >>> + && GET_CODE (*op0) == ZERO_EXTEND >>> + && GET_CODE (XEXP (*op0, 0)) == SUBREG >>> + && GET_MODE (XEXP (*op0, 0)) == QImode >>> + && GET_MODE (SUBREG_REG (XEXP (*op0, 0))) == SImode >>> + && SUBREG_BYTE (XEXP (*op0, 0)) == 0 >>> + && *op1 == const0_rtx) >>> + *op0 = gen_rtx_AND (SImode, SUBREG_REG (XEXP (*op0, 0)), >>> + GEN_INT (255)); >>> + >> >> This is wrong for big-endian code. You should use subreg_lowpart_p to >> check the subreg expression (after you've checked that you do have a >> subreg, of course). >> >> R. >> > > Hi Richard, thanks for catching that. I've updated the patch, and > cross-tested again under both arm/armeb-Linux. > > I changed the testcase to use -march=armv6t2 instead of armv6, as the > latter makes the testcase FAIL when configured as --with-mode=thumb. > > Is this now okay? > The patch to arm.c is ok, but the change to the test is not as it will cause problems with multilib testing. A better fix is to skip the test if the target is thumb1. The other test needs a similar check as it seems to expect a movs instruction. R. > Thanks, > Chung-Lin > <uxtb-cmp.diff>
