[llvm-commits] [PATCH, PR-13709]: Remove unnecessary SRL instruction to access high sub-reg of a register pair.

[Sebastian Pop]

On Mon, Aug 27, 2012 at 2:24 PM, Sebastian Pop <spop at codeaurora.org> wrote:
> Hi Pranav,
>
> On Mon, Aug 27, 2012 at 1:20 PM, Pranav Bhandarkar
> <pranavb at codeaurora.org> wrote:
>
>> +static cl::opt<bool> DisableOptRightShift32("disable-hexagon-optrightshift32",
>> +    cl::Hidden, cl::ZeroOrMore, cl::init(false),
>> +    cl::desc("Disable Optimization of LSR 32."));
>
> Please remove the dot at the end of a flag description.
> Why do we need a flag to disable this transform?
> Isn't this always a win?
>
>> +      // %vregDoubleReg1 = LSRd_ri %vregDoubleReg0, 32
>> +      // %vregIntReg = COPY %vregDoubleReg1:subreg_loreg.
>> +      // and convert into
>> +      // %vregIntReg = COPY %VregDoubleReg0:subreg_hireg.
>
> s/VregDoubleReg0/vregDoubleReg0/
>
>> +      if (!DisableOptRightShift32 &&
>> +            MI->getOpcode() == Hexagon::LSRd_ri) {
>
> the indentation of this doesn't look correct.
>
> Your patch looks good otherwise. Let's wait a bit for another review.

Pranav, go ahead and commit the amended version of your patch.

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum