[PATCH] TargetLowering: n * r where n > 2 should be an illegal addressing mode

Richard Sandiford rsandifo at linux.vnet.ibm.com
Fri Feb 14 02:46:29 PST 2014


Tom Stellard <tom at stellard.net> writes:
> On Thu, Feb 13, 2014 at 08:21:04AM -0800, Tom Stellard wrote:
>> From: Tom Stellard <thomas.stellard at amd.com>
>> 
>
> Please ignore this patch.  Scaled addressing modes are more
> common than I though, so I don't think this is correct.

I agree there's a problem though.  I hit the same thing before r187497:

    [SystemZ] Implement isLegalAddressingMode()
    
    The loop optimizers were assuming that scales > 1 were OK.  I think this
    is actually a bug in TargetLoweringBase::isLegalAddressingMode(),
    since it seems to be trying to reject anything that isn't r+i or r+r,
    but it has no default case for scales other than 0, 1 or 2.  Implementing
    the hook for z means that z can no longer test any change there though.

The comment says:

  // Only support r+r,

so I think it really was trying to disallow scaled addresses.  The switch
doesn't make much sense otherwise.  E.g. the gymnastics the scale==2 case
goes through:

    if (AM.HasBaseReg || AM.BaseOffs)  // 2*r+r  or  2*r+i is not allowed.
      return false;
    // Allow 2*r as r+r.
    break;

If scaled addresses are supported then the set of valid scales are going
to be target-dependent and need a target-specific override, so TBH I think
your patch is correct.  I was going to post the same thing originally but
implementing the z hook meant I no longer had a testcase.

Thanks,
Richard




More information about the llvm-commits mailing list